Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add all missing docstrings and typing hints, improve docs, fix some but not all bugs #39

Merged

Conversation

brainelectronics
Copy link
Owner

Added

Changed

  • Reordered modules of API documentation
  • data_as_registers and data_as_bits of common.py removed
  • Send illegal function code 0x01 if a register other than coil or holding register is requested to be set
  • Simplified _process_write_access logic of tcp.py

Fixed

…ue signing in TCP and Serial modules, contribute to #23
@brainelectronics brainelectronics added the bug Something isn't working label Dec 17, 2022
@brainelectronics brainelectronics added the documentation Improvements or additions to documentation label Dec 17, 2022
@brainelectronics brainelectronics added this to the Documentation milestone Dec 17, 2022
@brainelectronics brainelectronics self-assigned this Dec 17, 2022
@brainelectronics
Copy link
Owner Author

@beyonlo here are some new bugfixes for you to test in 2.1.0-rc15.dev39 😄 and some extended documentation for register usage as well https://micropython-modbus--39.org.readthedocs.build/en/39/USAGE.html#register-usage

@beyonlo
Copy link

beyonlo commented Dec 18, 2022

Hi @brainelectronics Thank you for this new release!!

some extended documentation for register usage as well https://micropython-modbus--39.org.readthedocs.build/en/39/USAGE.html#register-usage

I liked so much the extended docs. I think is missing (as I commented in #24): The 0x05 write_single_coil talk about how to write single COIL just on the first position, but do not talk about how to write a COIL in a different position than the first one, like as the second position and so on.

Tests reports:

register_definitions = {
    "COILS": {
        "RESET_REGISTER_DATA_COIL": {
            "register": 42,
            "len": 1,
            "val": 0
        },
        "RESET_REGISTER_DATA_COIL_2": {
            "register": 45,
            "len": 1,
            "val": 1
        },
        "EXAMPLE_COIL": {
            "register": 123,
            "len": 16,
            "val": [1,0,0,1,1,0,1,1,1,1,1,1,1,1,0,1]
        },
        "EXAMPLE_COIL_MIXED": {
            "register": 124,
            "len": 3,
            "val": [1,1,1],
            "description": "Example COILS registers with length of 2, Coils (setter+getter) [0, 1]",
            "range": "",
            "unit": ""
        }
    },
    "HREGS": {
        "EXAMPLE_HREG": {
            "register": 93,
            "len": 1,
            "val": 19
        },
        "ANOTHER_EXAMPLE_HREG": {
            "register": 94,
            "len": 9,
            "val": [29, 38, 0, 1600, 2150, 5067, 2564, 8450, 3456],
            "description": "Example HREGS registers with length of 3, Hregs (setter+getter) [0, 65535]",
            "range": "",
            "unit": ""
        }
    },
    "ISTS": {
        "EXAMPLE_ISTS": {
            "register": 67,
            "len": 1,
            "val": 0
        }
    },
    "IREGS": {
        "EXAMPLE_IREG": {
            "register": 10,
            "len": 2,
            "val": 60001
        }
    }
}

Slave:

$ mpremote run tcp_client_example.py
Waiting for WiFi connection...
Waiting for WiFi connection...
Waiting for WiFi connection...
Connected to WiFi.
('192.168.1.4', '255.255.255.0', '192.168.1.1', '192.168.1.1')
Setting up registers ...
Register setup done
Running ModBus version: 2.1.0-rc15.dev39
Serving as TCP client on 192.168.1.4:502
  1. "Enable reading more than 8 coils in a row, see Reading coils is limited to 8 bits #36"

Not fixed - still have errors, details below:

Master:

>>> from umodbus.tcp import TCP as ModbusTCPMaster
>>> host = ModbusTCPMaster(slave_ip='192.168.1.4', slave_port=502, timeout=5)
>>> from umodbus import version
>>> version.__version__
'2.1.0-rc15.dev39'
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=1)
[True] <---- OK
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=4)
[False, False, False, True] <---- OK
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=6)
[False, False, False, False, False, True] <---- OK
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=9)
[False, False, False, False, False, False, False, True] <---- NOT OK, coil_qty=9, but return just 8 COILS
>>> host.read_coils(slave_addr=10, starting_addr=45, coil_qty=16)
[False, False, False, False, False, False, False, True] <---- NOT OK, coil_qty=16, but return just 8 COILS
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=4) <---- NOT OK, Python error
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/umodbus/tcp.py", line 364, in read_coils
  File "/lib/umodbus/functions.py", line 394, in bytes_to_bool
ValueError: invalid format specifier
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=1) <---- NOT OK, Python error
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/umodbus/tcp.py", line 364, in read_coils
  File "/lib/umodbus/functions.py", line 394, in bytes_to_bool
ValueError: invalid format specifier
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=10) 
[True, False, False, True, True, False, True, True, True, True, True, True, True, True, False, True] <---- NOT OK, coil_qty=10, but return just 16 COILS
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=8) 
[True, False, False, True, True, False, True, True, True, True, True, True, True, True, False, True] <---- NOT OK, coil_qty=8, but return just 16 COILS
>>> 
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=1)
[True, True, True] <---- NOT OK, coil_qty=1, but return 3 COILS
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=8)
[False, False, False, False, False, True, True, True] <---- OK
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=16)
[False, False, False, False, False, True, True, True] <---- NOT OK, coil_qty=16, but return just 8 COILS
>>> 
  1. Writing multiple coils in TCP, see Bug: error when trying to write multiple coils - host.write_multiple_coils() #22

Works, but have some errors:

a) note about that before write multiple COILs, is reading from in normal way: from left to right from register definitions, but after is wrote, the read start inverse read, from right to left.
b) note that when write a list of COILs that with less COILs than in register_definitions, the other COILs is losted.

Master:

>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=1)
[True, True, True] <-- read_coils quantity not works, but this test is about  writing multiple coils
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=8)
[False, False, False, False, False, True, True, True]
>>> host.write_multiple_coils(slave_addr=10, starting_address=124, output_values=[1,0])
True <--- OK - No errors
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=8)
[False, False, False, False, False, False, False, True] <--- wrote OK - but look that lost that third COIL that was true.
>>> 
>>> host.write_multiple_coils(slave_addr=10, starting_address=124, output_values=[1,0,1,1,1])
True <--- OK - No errors
>>> host.read_coils(slave_addr=10, starting_addr=124, coil_qty=1)
[True, True, True, False, True] <--- OK - Values changed to correct value!
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/umodbus/tcp.py", line 364, in read_coils
  File "/lib/umodbus/functions.py", line 394, in bytes_to_bool
ValueError: invalid format specifier
>>> 
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=16)
[True, False, False, True, True, False, True, True, True, True, True, True, True, True, False, True]
>>> host.write_multiple_coils(slave_addr=10, starting_address=123, output_values=[1,1,0,1,1,0,1,1,1,1,1,1,1,1,0,1])
True <--- OK - No errors
>>> host.read_coils(slave_addr=10, starting_addr=123, coil_qty=16)
[True, False, True, True, True, True, True, True, True, True, False, True, True, False, True, True] <--- OK - Values changed to correct value, but look that after wrote happen a time, starting to read from inverse list.
  1. Writing multiple registers in TCP, see Bug: error when trying to write multiple registers - host.write_multiple_registers() #23

Master:
Works, but have some errors:

a) note that, like as happen with write_multiple_coils, writing multiple registers, when the list of registers that will write has less elements than in register_definitions, the all others registers is losted.

>>> host.read_holding_registers(slave_addr=10, starting_addr=94, register_qty=50, signed=False)
(29, 38, 0, 1600, 2150, 5067, 2564, 8450, 3456)
>>> host.write_multiple_registers(slave_addr=10, starting_address=94, register_values=[2, -4, 6, -256, 1024], signed=True)
True <--- OK wrote with sucess
>>> host.read_holding_registers(slave_addr=10, starting_addr=94, register_qty=50, signed=False)
(2, 65532, 6, 65280, 1024) <--- OK, but the registers 99, 100, 101 and 102 was losted
>>> 

Good to see that functions 0x0F write_multiple_coils and 0x10 write_multiple_registers starting works! Still there are problems as reported above, but was a good start!

Thank you very much!

@beyonlo
Copy link

beyonlo commented Dec 18, 2022

@brainelectronics more two things to report in this new version:

When I was to try to do that tests over ModBus RTU, happen this error:

$ mpremote run rtu_client_example.py
Running ModBus version: 2.1.0-rc15.dev39
Traceback (most recent call last):
  File "<stdin>", line 109, in <module>
AttributeError: 'ModbusRTU' object has no attribute 'process'

So, all tests above was done using only the ModBus TCP.

In this new version when I install using upip, it try to install micropython-urequests as well, but do not works. So, I continue to install like always done: using just urequests and works.

>>> upip.index_urls = ['https://test.pypi.org/pypi']
>>> upip.install('micropython-modbus')
Installing to: /lib/
Installing micropython-modbus 2.1.0rc15.dev39 from https://test-files.pythonhosted.org/packages/af/fe/4373dcec053064ed816ff5e4ce25dad7921e333a333a45ed863d085aac03/micropython-modbus-2.1.0rc15.dev39.tar.gz
Error installing 'micropython-urequests': Package not found, packages may be partially installed
>>> 

Copy link
Owner Author

@brainelectronics brainelectronics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brainelectronics
Copy link
Owner Author

Hi @beyonlo, as discussed in #22 (comment) a COIL has a length of 1. If you define a COIL 98 with a length of 1, you can only set 98. If you define a length of 2 then coils 98+99 are configured. Due to bug #35 you can only set all coils you defined at once. But if you define a coil 98 with length 1 and another coil 99 with a length of 1 you can modify each coil without affecting any others by write singel coil function 0x05. But with this temporary trick you can not read 98+99 with read_coils(slave_addr=10, starting_addr=98, coil_qty=2) see also #15 (comment)

Regarding the not implemented fix of #36: yes see also #38

Missing Modbus RTU process function, bugfix is on the way

@brainelectronics
Copy link
Owner Author

Works, but have some errors:
a) note about that before write multiple COILs, is reading from in normal way: from left to right from register definitions, but after is wrote, the read start inverse read, from right to left.
b) note that when write a list of COILs that with less COILs than in register_definitions, the other COILs is losted.

Remarka will be fixed with #38, remark b will be fixed with #35

@brainelectronics brainelectronics merged commit 00f5916 into develop Dec 27, 2022
@brainelectronics brainelectronics deleted the feature/add-all-missing-docstrings-and-typing-hints branch December 27, 2022 15:30
@beyonlo
Copy link

beyonlo commented Dec 28, 2022

@brainelectronics thank you very much for the clarifications! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants